feat: add telemetry scheduler #1107
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1107 +/- ##
==========================================
- Coverage 86.93% 86.79% -0.14%
==========================================
Files 60 62 +2
Lines 5555 6067 +512
==========================================
+ Hits 4829 5266 +437
- Misses 540 585 +45
- Partials 186 216 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
giortzisg
force-pushed
the
feat/telemetry-scheduler
branch
from
6c1fff1 to
2e16e4f
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
internal/telemetry/scheduler.go
Outdated
| } | ||
|
|
||
| func (s *Scheduler) processNextBatch() { | ||
| s.mu.Lock() |
There was a problem hiding this comment.
Do we even need a mutex? Is it even possible for this function to be called concurrently by 2 different goroutines?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
giortzisg
force-pushed
the
feat/telemetry-scheduler
branch
from
663a9eb to
cda84db
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| type logJSON struct { | ||
| Timestamp *float64 `json:"timestamp,omitempty"` | ||
| TraceID string `json:"trace_id,omitempty"` | ||
| Level string `json:"level"` | ||
| Severity int `json:"severity_number,omitempty"` | ||
| Body string `json:"body,omitempty"` | ||
| Attributes map[string]protocol.LogAttribute `json:"attributes,omitempty"` | ||
| } |
There was a problem hiding this comment.
Hmm I see the reason why we need this type now, e.g. Timestamp is a time.Time but you want to serialize it as float64, etc.
In Rust (serde) you could do this using serialize_with: https://serde.rs/field-attrs.html#serialize_with 😎
| envItem, err := item.ToEnvelopeItem() | ||
| if err != nil { | ||
| debuglog.Printf("error converting item: %v", err) | ||
| debuglog.Printf("error converting item to envelope: %v", err) |
There was a problem hiding this comment.
| debuglog.Printf("error converting item to envelope: %v", err) | |
| debuglog.Printf("error converting item to envelope item: %v", err) |
or
| debuglog.Printf("error converting item to envelope: %v", err) | |
| debuglog.Printf("error converting item: %v", err) |
I guess
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Description
Add telemetry scheduler implementation and integrate with the client. The current PR also includes changes on the transport and envelope layer.
The proper solution for batching under the scheduler is to abstract each event type, so that it knows how to convert itself to an envelope item (noted as EnvelopeItemConvertible in the PR), and then the scheduler would just create envelopes using
[]EnvelopeItemConvertible. This partially fixes the issue, since currently everything is anchored around theEventtype. We should also abstract all event types to implementEnvelopeItemConvertiblein the future.Notes
EnvelopeConvertibletoEnvelopeItemConvertibleintroduced in feat: add new envelope transport #1094. For the sake of clarity I added the changes here and not on the other PR. Same applies withToEnvelopeEnvelopeHeader. Left some comments on the implementation.Issues